Skip to content

feat: Replace Zitadel with Logto as OIDC identity provider#415

Merged
Strehk merged 11 commits intomainfrom
auth/replace-zitadel-with-logto
Apr 6, 2026
Merged

feat: Replace Zitadel with Logto as OIDC identity provider#415
Strehk merged 11 commits intomainfrom
auth/replace-zitadel-with-logto

Conversation

@Strehk
Copy link
Copy Markdown
Member

@Strehk Strehk commented Apr 5, 2026

Summary

  • Replace Zitadel with Logto as the OIDC identity provider, updating OIDC discovery, token validation, claim normalization, and role extraction
  • Integrate Logto Account Center deep-links into the my-account page, allowing users to change email, password, username, passkeys, MFA, and backup codes directly from the app with redirect-back toast notifications
  • Implement Logto-compatible impersonation via the two-step token exchange flow (Management API subject tokens + OIDC token exchange)
  • Add M2M credentials config (OIDC_M2M_CLIENT_ID, OIDC_M2M_CLIENT_SECRET, OIDC_M2M_RESOURCE) for Management API access
  • Surface MFA status, SSO/social identities, and password status from Logto Custom JWT claims in the my-account page
  • Update documentation (README, CLAUDE.md, .env.example) to reflect Logto as the recommended provider with setup guides

Test plan

  • Verify login/logout flow works with Logto OIDC
  • Verify my-account page displays user info, MFA status, connected identities
  • Verify each Account Center deep-link (email, password, username, passkeys, authenticator app, backup codes) redirects to Logto and back with success toast
  • Verify user impersonation works (start + stop) with M2M credentials configured
  • Verify impersonation banner shows correct original/impersonated user
  • Verify role-based access control still works with Logto role format
  • Run bun run check — no type errors

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Enhanced account settings UI: passkeys, authenticator app, backup codes, MFA status, connected SSO identities, contextual success toasts.
    • Deep-link support to account center and improved account redirect behavior.
    • Temporary migration notice flow with informational FAQ shown during login/account migration.
  • Expanded Data

    • Additional user profile fields surfaced (nullable name fields, password/mfa indicators, social/sso identity lists).
  • Documentation

    • OIDC guidance updated to recommend Logto and refreshed example env/README/WARP/CLAUDE notes.

Strehk and others added 6 commits April 4, 2026 23:48
- Adapt OIDC token validation to handle Logto's opaque and JWT access tokens
- Add claim normalization for Logto's different field names (username, name)
- Extract roles from JWT access tokens via Custom JWT claims
- Support OIDC_RESOURCE config for Logto API resource (JWT access tokens)
- Make OIDCUser fields (family_name, given_name, preferred_username) optional
- Update upsertSelf to use ctx.oidc.user instead of userinfo endpoint
- Handle Logto role objects ({name, id, ...}) in addition to string arrays
- Remove Zitadel-specific token introspection, token exchange types, and error messages
- Add Mailpit SMTP credentials for Logto email connector
- Update mock OIDC landing page with new roles claim format
- Add migration scripts for Zitadel→Logto user ID remapping

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the static login information card with an interactive account
management UI that deep-links to Logto's Account Center for credential
changes (email, password, username, passkeys, authenticator app, backup
codes). Display MFA status, connected social/SSO identities, and
password state from Logto Custom JWT claims. Improve responsive layout
with two-column grid on larger screens.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update all documentation to recommend Logto instead of Zitadel,
remove Zitadel-specific OIDC scopes and role claims from .env.example.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cs to .env.example

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Logto requires a two-step impersonation flow: first create a subject
token via the Management API, then exchange it at the OIDC token
endpoint. Also decode the resource-scoped JWT directly instead of
calling userinfo (which rejects resource-scoped tokens), and look up
profile claims from the database.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Strehk Strehk added the PR: Feature New feature label Apr 5, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 5, 2026

📝 Walkthrough

Walkthrough

Replaces ZITADEL-focused OIDC guidance with Logto, adds Logto M2M/subject-token impersonation flow and claim normalization, relaxes several profile fields to nullable, adds Prisma migration/seed scripts, a migration-notice UI/flow, i18n updates, and minor CI/config adjustments.

Changes

Cohort / File(s) Summary
Env & Docs
\.env.example, CLAUDE.md, README.md, WARP.md
Switched recommended OIDC provider to Logto; updated example discovery URL, scopes, OIDC_ROLE_CLAIM, added optional M2M env vars and PUBLIC_OIDC_MIGRATION_NOTICE placeholder.
CI / Actions / Compose
.github/actions/setup-bun/action.yml, .github/workflows/ci.yml, dev.docker-compose.yml
Bumped actions/cache/actions/checkout to v5; adjusted Mailpit SMTP auth and dev mock OIDC claims to top-level roles.
I18n
messages/en.json, messages/de.json
Added account/security strings and migration-notice copy (backup codes, passkeys, MFA, SSO identities, migration FAQ/title/body, etc.).
Mock OIDC
mock-oidc-landingpage.html
Changed mock preset roles shape from namespaced object to top-level roles array.
Prisma / Migration Scripts
prisma/seed/createMissingLogtoUsers.ts, prisma/seed/migrateOidcSubs.ts
Added scripts to create/migrate Logto users and remap FK references using transactional updates; require credentials/input.
GraphQL Schema
schema.graphql
Made name/username fields nullable on impersonation/offline types; added hasPassword, mfaVerificationFactors, socialIdentities, ssoIdentities, and OfflineUserSsoIdentity.
OIDC Core & Context
src/api/services/OIDC.ts, src/api/context/oidc.ts
Added claim normalization, relaxed OIDC user validation to sub+email, JWKS/config accessors, new M2M subject-token impersonation flow (get M2M token → create subject token → token exchange), improved role parsing, and jwtVerify-based impersonation verification.
Resolvers / Auth Logic
src/api/resolvers/modules/auth.ts, src/api/resolvers/modules/impersonation.ts, src/api/resolvers/modules/user.ts
Adjusted resolvers to nullable/new OfflineUser fields; removed some userinfo fetch paths; modified impersonation token-exchange params and upsert mapping to tolerate nullable profile fields.
Config Schema
src/config/private.ts, src/config/public.ts
Added OIDC_RESOURCE, OIDC_M2M_CLIENT_ID, OIDC_M2M_CLIENT_SECRET, OIDC_M2M_RESOURCE, PUBLIC_OIDC_ACCOUNT_URL, PUBLIC_OIDC_MIGRATION_NOTICE; several URL/string fields now coerce ''undefined.
Frontend & Queries
src/lib/queries/fastUserQuery.ts, src/routes/(authenticated)/my-account/+page.server.ts, src/routes/(authenticated)/my-account/+page.svelte
Extended fastUserQuery and my-account server payload; added account-center deep-links and success flags; updated my-account UI to surface MFA/passkey/backup-code status, SSO identities, conditional UI, and localized toasts.
Migration Notice UI & Flow
src/lib/constants/migrationNotice.ts, src/routes/(authenticated)/+layout.server.ts, src/routes/auth/migration-notice/*
Added migration notice constants, gate in authenticated load to redirect when notice enabled, and new migration-notice page/action to acknowledge cookie and start OIDC sign-in.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant ManagementAPI as Logto Management API
    participant OIDCProvider as OIDC Provider
    participant Database

    rect rgba(100,150,200,0.5)
        Note over Server,ManagementAPI: New Logto M2M subject-token impersonation flow
        Client->>Server: Impersonation request (impersonation JWT)
        Server->>Server: jwtVerify()/decode impersonation token
        Server->>Database: fetch user by sub
        Server->>Server: validate actor claim vs session
        Server->>ManagementAPI: request M2M access token (client_id/secret)
        ManagementAPI-->>Server: m2m_access_token
        Server->>ManagementAPI: create subject token (POST /api/subject-tokens with sub)
        ManagementAPI-->>Server: subject_token
        Server->>OIDCProvider: token exchange using subject_token (+resource if configured)
        OIDCProvider-->>Server: exchanged tokens
        Server->>Database: record/session for impersonation
    end
Loading
sequenceDiagram
    participant Client
    participant Server
    participant OIDCIssuer as OIDC Issuer
    participant Database

    rect rgba(100,200,100,0.5)
        Note over Server,OIDCIssuer: Claim normalization & relaxed profile handling
        Client->>Server: Login / token refresh
        Server->>OIDCIssuer: verify id_token, decode access_token
        OIDCIssuer-->>Server: id_token + access_token claims
        Server->>Server: normalizeOIDCClaims() (username→preferred_username, name→given/family)
        Server->>Server: ensure sub + email present
        Server->>Database: upsert user (nullable name fields accepted)
        Database-->>Server: user row created/updated
        Server->>Client: session / offline user response including new fields
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

feature

Suggested reviewers

  • m1212e

Poem

🐰 I hopped from Zitadel's tidy glade,

to Logto fields where tokens parade.
I normalized claims, and stitched roles anew,
migrated users, cookies, and UI view—
now accounts skip merrily in the dew.

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Boy Scout Scope ⚠️ Warning PR includes undocumented opportunistic changes: GitHub Actions version updates (cache@v4→v5, checkout@v4→v5) and Mailpit SMTP configuration changes unrelated to the primary Zitadel→Logto migration, with no Boy Scout changes section in the PR description. Add a Boy Scout changes section to the PR description documenting these opportunistic updates with justification, or extract them into a separate PR to maintain clear scope boundaries.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: Replace Zitadel with Logto as OIDC identity provider' clearly and concisely summarizes the primary change—migrating from Zitadel to Logto as the OIDC provider. It accurately reflects the main focus of the changeset across configuration, documentation, and code modifications.
Pr Label Required ✅ Passed The pull request carries the 'PR: Feature' label, which is one of the accepted release-category labels specified in the requirements.
German Gender-Inclusive Language ✅ Passed All 27 new German translations in messages/de.json comply with gender-inclusive language guidelines. New entries contain technical terms, verbs, and informational content without person-related nouns requiring gender-inclusive notation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch auth/replace-zitadel-with-logto

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Strehk Strehk requested a review from m1212e April 5, 2026 00:52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Nitpick comments (1)
src/routes/(authenticated)/my-account/+page.server.ts (1)

62-65: Build the fallback account URL via URL instead of string concatenation.

The current concatenation can produce malformed paths in edge cases (for example, trailing slash/path variants). Using URL keeps this robust.

♻️ Suggested refactor
-	const accountCenterUrl =
-		configPublic.PUBLIC_OIDC_ACCOUNT_URL ??
-		configPublic.PUBLIC_OIDC_AUTHORITY.replace(/\/oidc\/?$/, '') + '/account';
+	const accountCenterUrl =
+		configPublic.PUBLIC_OIDC_ACCOUNT_URL ??
+		(() => {
+			const authority = new URL(configPublic.PUBLIC_OIDC_AUTHORITY);
+			authority.pathname = authority.pathname.replace(/\/oidc\/?$/, '/');
+			return new URL('account', authority).toString();
+		})();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/`(authenticated)/my-account/+page.server.ts around lines 62 - 65,
The fallback accountCenterUrl building currently uses string concatenation which
can create malformed URLs; update the logic that sets accountCenterUrl to use
the URL API instead of concatenation: if configPublic.PUBLIC_OIDC_ACCOUNT_URL is
present use it, otherwise construct a new URL('/account',
configPublic.PUBLIC_OIDC_AUTHORITY.replace(/\/oidc\/?$/, '')) (or equivalent
using new URL with eventUrl.origin as needed) so paths are joined correctly;
keep the existing accountRedirectUrl (`${eventUrl.origin}/my-account`) behavior
unchanged or likewise build it via new URL('/my-account', eventUrl.origin) for
consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.env.example:
- Around line 39-43: The .env.example redirect URI is incorrect: update the
documentation to instruct registering the redirect URI as
<your-domain>/auth/login-callback to match the actual callback used by the code
(startSignin() in src/api/services/OIDC.ts); change the text that currently says
<your-domain>/auth/callback to <your-domain>/auth/login-callback so operators'
provider redirect URI matches the startSignin() behavior.

In `@prisma/seed/createMissingLogtoUsers.ts`:
- Around line 37-89: The Logto user creation (the POST to
`${LOGTO_ENDPOINT}/api/users` inside the for (const user of users) loop
producing response/body and logtoId) is non-idempotent and runs before the
Prisma transaction (db.$transaction) so failures leave orphaned Logto users;
change the flow to perform local preflight and use an existing Logto user if
present or move the Logto creation until after the transaction succeeds, or if
you must create it before the DB changes implement a compensating delete of that
Logto user (using logtoId) inside the catch/failure path of the db.$transaction
block; reference the variables/functions response, body, logtoId,
db.$transaction, tx.user.create, and tx.user.delete to locate where to add the
preflight check or compensating delete.
- Line 61: The progress logs in createMissingLogtoUsers.ts currently print raw
PII (user.email and identity provider IDs) to the console (see the
console.error/console.log calls that include user.email and any provider ID
values); change those log statements to avoid emitting full emails or stable
provider IDs by replacing them with a non-PII identifier (e.g., user.id, a
truncated/masked email like user.email.replace(/(.{2}).+(@.+)/, '$1***$2'), or a
short hash of the email) and remove or mask any provider ID strings before
logging, preserving the original message context (body.message or JSON) but not
the raw PII.
- Around line 58-65: Add a runtime type-narrowing guard before reading body.id:
after parsing body from response, verify body is a non-null object and that
typeof body.id === 'string' (and optionally typeof body.message === 'string' if
you access it); if the check fails, log an error including the raw body and
continue the loop instead of assigning to logtoId or performing DB operations.
Update the block around the existing response/body logic (where response.json()
is assigned and logtoId is set) to enforce this guard and handle the
invalid-response path safely.

In `@prisma/seed/migrateOidcSubs.ts`:
- Line 57: The stdout uses raw identifiers (mapping.identifier and
mapping.zitadel_id) which may expose emails and provider subject IDs; replace
those console.log calls in migrateOidcSubs.ts with masked versions (e.g., show
only first 1–3 chars + redacted placeholder or hash) or emit counts-only
messages; update both the log that currently prints mapping.identifier and
mapping.zitadel_id and the similar log later in the file to use a
mask/obfuscation helper (e.g., maskEmail/maskId) or a summarized message so no
full email or subject ID is written to stdout.
- Around line 51-87: Before calling tx.user.create with mapping.logto_id, detect
if a target user already exists and handle it instead of blindly creating—use
db.user.findUnique or tx.user.findUnique to check for a row with id ===
mapping.logto_id; if found, either move FK references to that existing target
and delete the source (perform the same FK UPDATE loop and source delete) or
skip this mapping and increment skipped, and only create the temporary/migrating
row when no target exists; update the logic in the block around tx.user.create /
tx.$executeRawUnsafe / tx.user.delete / tx.user.update to branch on the
existence of mapping.logto_id to avoid duplicate-key failures and half-completed
transactions.

In `@README.md`:
- Line 51: Fix the typo and wording in the README string that mentions the
example compose file: change "directoy" to "directory" and capitalize/format
"docker compose" as "Docker Compose" in the sentence that reads "You can find an
example docker compose file in the [example](./example/) directoy." Ensure the
updated sentence references the example directory and uses "Docker Compose"
consistently.

In `@schema.graphql`:
- Around line 2817-2819: OfflineUser now marks family_name and given_name
nullable, but fastUserQuery spreads OfflineUser into layout data and components
may assume non-null names; update the downstream consumers to defensively handle
nulls by coalescing or conditional-rendering: locate usages of
OfflineUser/family_name and OfflineUser/given_name in fastUserQuery results and
in components receiving layout data (search for fastUserQuery, layoutData
spread, and components that read family_name/given_name), then replace direct
access with safe fallbacks (e.g., use displayName = family_name ?? given_name ??
email ?? '' or conditionally render name blocks) so no runtime errors occur when
those fields are null while leaving ImpersonatedUser/ImpersonationUser handling
unchanged.

In `@src/api/context/oidc.ts`:
- Around line 123-137: Replace the inline role-parsing blocks that push into
OIDCRoleNames (the branches that read rolesRaw and the Object.keys path) with a
shared helper (e.g., parseOidcRoles or extractOidcRoleNames) that accepts
unknown rolesRaw and the allowed oidcRoles set/array and returns a string[] of
validated role names; inside the helper treat the input as unknown, narrow types
(Array.isArray, typeof element === 'string', element is object with typeof
element.name === 'string'), do NOT use any casts, extract the name string when
present, and filter the results against the allowed oidcRoles so only
'admin'|'member'|'service_user' are returned; replace both occurrences (user and
impersonated-user paths) to call this helper and append its returned values to
OIDCRoleNames.
- Around line 154-199: The code currently uses
decodeJwt(impersonationTokenSet.data.access_token) which does no signature
verification; replace this with a cryptographic verification using jwtVerify()
against the OIDC issuer JWKS (reuse the same JWKS fetching/verification logic
used for id_token verification in this file) to validate the impersonation
access_token before trusting its claims, then extract the verified payload and
use that to look up the DB user (symbols: decodeJwt -> jwtVerify,
impersonationTokenSet.data.access_token, issuer/JWKS fetch routine,
db.user.findUnique, impersonatedUser). Also ensure the actor check uses the
verified token payload (act.sub) and on verification failure or actor mismatch
delete the impersonation cookie (impersonationTokenCookieName), abort
impersonation and return the same token response path as currently implemented.

In `@src/api/resolvers/modules/auth.ts`:
- Around line 70-89: The code currently asserts custom JWT claims on
ctx.oidc.user into claims and casts values directly into hasPassword,
mfaVerificationFactors, ssoIdentities, and socialIdentities; add runtime
validation instead of blind casts by implementing small type guards (e.g.,
isBooleanClaim, isStringArrayClaim, isSsoIdentitiesArrayClaim that checks
objects have issuer and identityId strings) and use them when reading claims
from ctx.oidc.user/claims so that invalid shapes set the GraphQL fields to null
(or a safe default) rather than casting; alternatively, if you choose the
type-level fix, update the OIDCUser type to explicitly declare password, mfa,
sso_identities, and social_identities so the compiler enforces shapes at the
source (referencing ctx.oidc.user, claims, hasPassword, mfaVerificationFactors,
ssoIdentities, socialIdentities).

In `@src/api/resolvers/modules/impersonation.ts`:
- Around line 262-266: The resolver is accepting args.scope but never forwarding
it to performTokenExchange, so callers think down-scoping occurs while it is
ignored; update the impersonation resolver to pass args.scope into
performTokenExchange (e.g., performTokenExchange(ctx.oidc.tokenSet.access_token,
args.targetUserId, args.scope)) or alternatively remove the GraphQL scope
argument from the schema and resolver signature; look for usages of
performTokenExchange and args.scope in impersonation.ts and either forward
args.scope through the call or delete the scope argument and all references to
it.

In `@src/api/resolvers/modules/user.ts`:
- Around line 432-436: The current spreads use truthy checks (e.g.,
...(issuerUserData.family_name && { family_name: issuerUserData.family_name }))
which drop empty-string values; change these to nullish checks so intentional
empty values are preserved—replace each truthy guard for
issuerUserData.family_name, issuerUserData.given_name, and
issuerUserData.preferred_username with a nullish check (e.g., use
issuerUserData.family_name != null ? { family_name: issuerUserData.family_name }
: {} ) so only null/undefined are omitted while empty strings are allowed
through.

In `@src/api/services/OIDC.ts`:
- Around line 353-364: The token exchange builds tokenExchangeParams but omits
the actor_token fields so impersonation tokens are never bound; modify the
exchange to include actor_token and actor_token_type (use the existing
actorToken variable and set actor_token_type to
'urn:ietf:params:oauth:token-type:access_token') when actorToken is present, so
the request sent from the function that creates subjectToken and performs the
token exchange includes those fields and the resulting JWT contains the act
claim expected by the verification in src/api/context/oidc.ts.
- Around line 174-180: The decodeAccessTokenClaims helper currently returns
unverified claims from decodeJwt; stop merging those into session by either
verifying the access_token signature against the issuer JWKS before returning
claims (reuse the same JWKS verification logic used for id_token) or replace
usage with a server-side call to the userinfo endpoint to obtain authoritative
claims—update callers that merge claims into session to use the
verified/userinfo result instead of decodeAccessTokenClaims. Also fix
performTokenExchange to include the provided actorToken by adding an actor_token
entry to the tokenExchangeParams body so the token exchange request binds the
actor (preserve the existing subject_token behavior and include actor_token
alongside it).

In `@src/config/public.ts`:
- Line 11: Change the optional URL/string zod schemas to first normalize empty
strings to undefined using z.preprocess so empty env values are treated as
unset; specifically update the schemas for PUBLIC_OIDC_ACCOUNT_URL,
PUBLIC_SENTRY_DSN, PUBLIC_BADGE_GENERATOR_URL, PUBLIC_DOCS_URL (and in the other
config file for OIDC_M2M_RESOURCE and SENTRY_DSN) to use z.preprocess(v => v ===
"" ? undefined : v, z.string().url().optional() or z.string().optional() as
appropriate) so the existing nullish-coalescing logic (??) works and empty
strings no longer fail validation.

In `@src/routes/`(authenticated)/my-account/+page.svelte:
- Around line 193-263: The action anchors that render only an icon need
accessible names: update each <a class="btn btn-ghost btn-sm"
href={accountUrl(...)}> (used with accountUrl('username'), accountUrl('email'),
accountUrl('password'), accountUrl(hasPasskey ? 'passkey/manage' :
'passkey/add'), accountUrl(hasTotp ? 'authenticator-app/replace' :
'authenticator-app'), and accountUrl(hasBackupCodes ? 'backup-codes/manage' :
'backup-codes/generate')) to include either an appropriate aria-label (e.g.,
"Change username", "Change email", "Change password", "Manage passkeys" / "Add
passkey", "Replace authenticator app" / "Set up authenticator app", "Manage
backup codes" / "Generate backup codes") or add visually hidden text inside the
anchor so screen readers and keyboard users get descriptive labels; ensure
labels reflect the conditional text determined by hasPasskey, hasTotp, and
hasBackupCodes.

---

Nitpick comments:
In `@src/routes/`(authenticated)/my-account/+page.server.ts:
- Around line 62-65: The fallback accountCenterUrl building currently uses
string concatenation which can create malformed URLs; update the logic that sets
accountCenterUrl to use the URL API instead of concatenation: if
configPublic.PUBLIC_OIDC_ACCOUNT_URL is present use it, otherwise construct a
new URL('/account', configPublic.PUBLIC_OIDC_AUTHORITY.replace(/\/oidc\/?$/,
'')) (or equivalent using new URL with eventUrl.origin as needed) so paths are
joined correctly; keep the existing accountRedirectUrl
(`${eventUrl.origin}/my-account`) behavior unchanged or likewise build it via
new URL('/my-account', eventUrl.origin) for consistency.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3921efe3-6860-4a20-b750-3268391b70af

📥 Commits

Reviewing files that changed from the base of the PR and between f2587d1 and 574be1f.

📒 Files selected for processing (23)
  • .env.example
  • .github/actions/setup-bun/action.yml
  • .github/workflows/ci.yml
  • CLAUDE.md
  • README.md
  • WARP.md
  • dev.docker-compose.yml
  • messages/de.json
  • messages/en.json
  • mock-oidc-landingpage.html
  • prisma/seed/createMissingLogtoUsers.ts
  • prisma/seed/migrateOidcSubs.ts
  • schema.graphql
  • src/api/context/oidc.ts
  • src/api/resolvers/modules/auth.ts
  • src/api/resolvers/modules/impersonation.ts
  • src/api/resolvers/modules/user.ts
  • src/api/services/OIDC.ts
  • src/config/private.ts
  • src/config/public.ts
  • src/lib/queries/fastUserQuery.ts
  • src/routes/(authenticated)/my-account/+page.server.ts
  • src/routes/(authenticated)/my-account/+page.svelte

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 6, 2026

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 10 file(s) based on 11 unresolved review comments.

A stacked PR containing fixes has been created.

  • Stacked PR: #416
  • Files modified:
  • .env.example
  • README.md
  • src/api/context/oidc.ts
  • src/api/resolvers/modules/auth.ts
  • src/api/resolvers/modules/impersonation.ts
  • src/api/resolvers/modules/user.ts
  • src/api/services/OIDC.ts
  • src/config/private.ts
  • src/config/public.ts
  • src/routes/(authenticated)/my-account/+page.svelte

Time taken: 19m 24s

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Tade Strehk <git@strehk.eu>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/routes/(authenticated)/my-account/+page.svelte (1)

32-44: Consider preventing duplicate toast notifications on re-renders.

The $effect runs whenever data.accountUpdateSuccess changes. However, if the page component re-renders for other reasons while accountUpdateSuccess is still set (e.g., navigation back to this page), the toast may fire again unexpectedly. Consider tracking whether the toast was already shown for the current value.

♻️ Suggested improvement
+	let lastToastShown = $state<string | null>(null);
+
 	// Show toast for successful account center updates
 	$effect(() => {
 		const successMap: Record<string, () => string> = {
 			email: () => m.accountUpdateSuccessEmail(),
 			password: () => m.accountUpdateSuccessPassword(),
 			username: () => m.accountUpdateSuccessUsername(),
 			passkey: () => m.accountUpdateSuccessPasskey(),
 			mfa: () => m.accountUpdateSuccessMfa(),
 			'backup-codes': () => m.accountUpdateSuccessBackupCodes()
 		};
-		if (data.accountUpdateSuccess && successMap[data.accountUpdateSuccess]) {
+		if (data.accountUpdateSuccess && successMap[data.accountUpdateSuccess] && lastToastShown !== data.accountUpdateSuccess) {
 			toast.success(successMap[data.accountUpdateSuccess]());
+			lastToastShown = data.accountUpdateSuccess;
 		}
 	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/`(authenticated)/my-account/+page.svelte around lines 32 - 44, The
effect currently calls toast.success whenever data.accountUpdateSuccess is
truthy which can re-fire on unrelated re-renders; add a small local tracker
(e.g., a module-scoped variable like lastShownSuccess or a Set) outside the
$effect to remember the last shown value, then inside the $effect compare
data.accountUpdateSuccess to that tracker and only call
toast.success(successMap[... ]()) and update the tracker when the value is new;
ensure you still handle different string keys (email, password, username,
passkey, mfa, backup-codes) and optionally clear the tracker when
accountUpdateSuccess is cleared so future updates can show again.
src/api/services/OIDC.ts (1)

57-67: Name splitting heuristic may produce unexpected results for some name formats.

The logic splits name on the last space, assuming a "Given Family" order. This works for many Western names but may produce unexpected results for:

  • Names with multiple middle names (e.g., "John Paul George Ringo" → given: "John Paul George", family: "Ringo")
  • Single-word names or mononyms (correctly handled by the fallback)
  • Names in "Family Given" order (common in some cultures)

Since family_name and given_name are now optional, this is acceptable as a best-effort fallback when the provider only supplies a combined name claim.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` around lines 57 - 67, The current heuristic splits
normalized.name on the last space which can misassign multi-part names; change
the logic in OIDC.ts (the block that manipulates normalized.given_name and
normalized.family_name) to only split when parts.length === 2 (set given_name =
parts[0], family_name = parts[1]); for single-word names keep the existing
fallback (set given_name = normalized.name and family_name = normalized.name or
leave family_name undefined as your policy dictates), and for names with more
than two parts avoid applying the "last-space" split (e.g., set given_name =
normalized.name and leave family_name undefined) so you don't incorrectly map
multi-part names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.env.example:
- Line 249: The file .env.example is missing a trailing newline; edit the file
so that the final line "# PUBLIC_SHA=abc123" is terminated with a newline
character (i.e., add an empty line at EOF) to satisfy dotenv-linter and standard
POSIX text file conventions.

---

Nitpick comments:
In `@src/api/services/OIDC.ts`:
- Around line 57-67: The current heuristic splits normalized.name on the last
space which can misassign multi-part names; change the logic in OIDC.ts (the
block that manipulates normalized.given_name and normalized.family_name) to only
split when parts.length === 2 (set given_name = parts[0], family_name =
parts[1]); for single-word names keep the existing fallback (set given_name =
normalized.name and family_name = normalized.name or leave family_name undefined
as your policy dictates), and for names with more than two parts avoid applying
the "last-space" split (e.g., set given_name = normalized.name and leave
family_name undefined) so you don't incorrectly map multi-part names.

In `@src/routes/`(authenticated)/my-account/+page.svelte:
- Around line 32-44: The effect currently calls toast.success whenever
data.accountUpdateSuccess is truthy which can re-fire on unrelated re-renders;
add a small local tracker (e.g., a module-scoped variable like lastShownSuccess
or a Set) outside the $effect to remember the last shown value, then inside the
$effect compare data.accountUpdateSuccess to that tracker and only call
toast.success(successMap[... ]()) and update the tracker when the value is new;
ensure you still handle different string keys (email, password, username,
passkey, mfa, backup-codes) and optionally clear the tracker when
accountUpdateSuccess is cleared so future updates can show again.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e114b1ff-05e7-47b3-b36c-db81fa2742de

📥 Commits

Reviewing files that changed from the base of the PR and between 574be1f and f67bf35.

📒 Files selected for processing (10)
  • .env.example
  • README.md
  • src/api/context/oidc.ts
  • src/api/resolvers/modules/auth.ts
  • src/api/resolvers/modules/impersonation.ts
  • src/api/resolvers/modules/user.ts
  • src/api/services/OIDC.ts
  • src/config/private.ts
  • src/config/public.ts
  • src/routes/(authenticated)/my-account/+page.svelte
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/config/private.ts
  • src/api/resolvers/modules/user.ts
  • src/api/resolvers/modules/impersonation.ts
  • src/config/public.ts

Strehk and others added 3 commits April 6, 2026 14:29
…to session

Previously decodeAccessTokenClaims used jose's decodeJwt which returns
unverified claims. Now verifies the JWT signature against the issuer JWKS,
preventing unverified claims (e.g. roles) from being trusted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Shows an intermediate warning page before OIDC login when
PUBLIC_OIDC_MIGRATION_NOTICE is enabled, informing users about the
Zitadel-to-Logto migration with FAQ and instructions. Uses a
version-keyed cookie so the notice can be re-shown by bumping the
version. Disabled by default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add FAQ section for first-time registrants explaining the change
doesn't affect them. Add Zitadel/Logto links in the identity
provider explanation using @html rendering.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/api/services/OIDC.ts (1)

379-400: ⚠️ Potential issue | 🟠 Major

actor_token and actor_token_type are still missing from the token exchange request.

The past review flagged that actorToken is received but never included in tokenExchangeParams. This issue persists: line 371-377 extracts the actor for logging only, but the token exchange request (lines 386-400) omits actor_token and actor_token_type. Without these fields, the resulting impersonation JWT won't contain the act claim, breaking the audit trail of who initiated impersonation. The downstream verification in src/api/context/oidc.ts expects decoded.act to verify impersonation ownership.

🔧 Proposed fix to include actor_token in the exchange
 		const tokenExchangeParams: Record<string, string> = {
 			grant_type: 'urn:ietf:params:oauth:grant-type:token-exchange',
 			subject_token: subjectToken,
-			subject_token_type: 'urn:ietf:params:oauth:token-type:access_token'
+			subject_token_type: 'urn:ietf:params:oauth:token-type:access_token',
+			actor_token: actorToken,
+			actor_token_type: 'urn:ietf:params:oauth:token-type:access_token'
 		};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` around lines 379 - 400, The token exchange omits
the actor token fields so the impersonation JWT lacks the `act` claim; update
the code that builds tokenExchangeParams (in the same function that calls
createSubjectToken and reads actorToken) to add actor_token = actorToken and
actor_token_type = 'urn:ietf:params:oauth:token-type:access_token' when
actorToken is present (i.e., only attach these two keys if actorToken is
truthy), so the token exchange request includes the actor and downstream
verification of decoded.act will work.
.env.example (1)

253-253: ⚠️ Potential issue | 🟡 Minor

Add trailing newline at end of file.

The file is missing a trailing newline, which is flagged by dotenv-linter and is a POSIX convention for text files.

-# PUBLIC_SHA=abc123
+# PUBLIC_SHA=abc123
+
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.env.example at line 253, Add a trailing newline (LF) at the end of the
.env.example file so the final line containing the commented variable "#
PUBLIC_SHA=abc123" ends with a newline character; simply ensure the file
terminates with a newline to satisfy dotenv-linter and POSIX text-file
conventions.
🧹 Nitpick comments (1)
src/api/services/OIDC.ts (1)

57-67: Name-splitting heuristic may produce unexpected results for non-Western names.

The logic assumes the last whitespace-delimited word is the family name and everything before is the given name. This works for "John Doe" but fails for:

  • Single-word names (duplicates the name to both fields on line 64-65)
  • East Asian name order (family name first)
  • Names with particles ("Ludwig van Beethoven" → given: "Ludwig van", family: "Beethoven")

Since these fields are now optional and only used for display, consider documenting this limitation or simply preserving the original name field when parsing fails.

♻️ Suggested simplification for single-word names
 	if ((!normalized.family_name || !normalized.given_name) && normalized.name) {
 		const parts = normalized.name.trim().split(/\s+/);
 		if (parts.length >= 2) {
 			normalized.given_name = parts.slice(0, -1).join(' ');
 			normalized.family_name = parts[parts.length - 1];
 		} else {
-			normalized.given_name = normalized.name;
-			normalized.family_name = normalized.name;
+			// Single-word name: leave family_name undefined, use full name as given_name
+			normalized.given_name = normalized.name;
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` around lines 57 - 67, The current name-splitting in
OIDC (logic touching normalized.name, normalized.given_name,
normalized.family_name) assumes Western order and duplicates single-word names
into both given and family names; change it to only split when there are at
least two words and avoid duplicating the single-word case—i.e., if parts.length
>= 2 set normalized.given_name = parts.slice(0, -1).join(' ') and
normalized.family_name = parts[parts.length - 1], otherwise leave
normalized.given_name and normalized.family_name unset (or set family_name to
undefined) and rely on normalized.name for display; add a brief comment noting
the heuristic limitation instead of forcing potentially incorrect family/given
values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/api/services/OIDC.ts`:
- Around line 324-348: createSubjectToken currently derives the Management API
base URL directly from the issuer; change it to mirror getM2MAccessToken by
using configPrivate.OIDC_M2M_RESOURCE with a fallback of
`${issuer.replace(/\/oidc\/?$/, '')}/api`. Update createSubjectToken to compute
managementApiBase = configPrivate.OIDC_M2M_RESOURCE ??
`${issuer.replace(/\/oidc\/?$/, '')}/api` and then POST to the subject-tokens
endpoint relative to that base (e.g., `${managementApiBase}/subject-tokens`)
while keeping the existing headers, body, timeout, and error handling; reference
createSubjectToken, getM2MAccessToken, and configPrivate.OIDC_M2M_RESOURCE when
making the change.

---

Duplicate comments:
In @.env.example:
- Line 253: Add a trailing newline (LF) at the end of the .env.example file so
the final line containing the commented variable "# PUBLIC_SHA=abc123" ends with
a newline character; simply ensure the file terminates with a newline to satisfy
dotenv-linter and POSIX text-file conventions.

In `@src/api/services/OIDC.ts`:
- Around line 379-400: The token exchange omits the actor token fields so the
impersonation JWT lacks the `act` claim; update the code that builds
tokenExchangeParams (in the same function that calls createSubjectToken and
reads actorToken) to add actor_token = actorToken and actor_token_type =
'urn:ietf:params:oauth:token-type:access_token' when actorToken is present
(i.e., only attach these two keys if actorToken is truthy), so the token
exchange request includes the actor and downstream verification of decoded.act
will work.

---

Nitpick comments:
In `@src/api/services/OIDC.ts`:
- Around line 57-67: The current name-splitting in OIDC (logic touching
normalized.name, normalized.given_name, normalized.family_name) assumes Western
order and duplicates single-word names into both given and family names; change
it to only split when there are at least two words and avoid duplicating the
single-word case—i.e., if parts.length >= 2 set normalized.given_name =
parts.slice(0, -1).join(' ') and normalized.family_name = parts[parts.length -
1], otherwise leave normalized.given_name and normalized.family_name unset (or
set family_name to undefined) and rely on normalized.name for display; add a
brief comment noting the heuristic limitation instead of forcing potentially
incorrect family/given values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ddc72029-7aa3-439a-b1ef-07ed0b0b446d

📥 Commits

Reviewing files that changed from the base of the PR and between f67bf35 and a2d05b9.

⛔ Files ignored due to path filters (1)
  • src/assets/undraw/moving.svg is excluded by !**/*.svg
📒 Files selected for processing (9)
  • .env.example
  • messages/de.json
  • messages/en.json
  • src/api/services/OIDC.ts
  • src/config/public.ts
  • src/lib/constants/migrationNotice.ts
  • src/routes/(authenticated)/+layout.server.ts
  • src/routes/auth/migration-notice/+page.server.ts
  • src/routes/auth/migration-notice/+page.svelte
✅ Files skipped from review due to trivial changes (2)
  • src/lib/constants/migrationNotice.ts
  • messages/en.json

…tToken

Align createSubjectToken with getM2MAccessToken by using the
configurable OIDC_M2M_RESOURCE with fallback, instead of hardcoding
the base URL derivation from the issuer.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/api/services/OIDC.ts (1)

380-401: ⚠️ Potential issue | 🔴 Critical

actor_token still not included in token exchange request.

The actorToken parameter is received but never added to tokenExchangeParams. Per RFC 8693 and the comment on lines 385-386, including actor_token and actor_token_type is necessary to bind the act claim in the resulting JWT. Without this, downstream verification in src/api/context/oidc.ts (which checks decoded.act) cannot confirm who initiated the impersonation.

🔒 Proposed fix to include actor_token
 		const tokenExchangeParams: Record<string, string> = {
 			grant_type: 'urn:ietf:params:oauth:grant-type:token-exchange',
 			subject_token: subjectToken,
-			subject_token_type: 'urn:ietf:params:oauth:token-type:access_token'
+			subject_token_type: 'urn:ietf:params:oauth:token-type:access_token',
+			actor_token: actorToken,
+			actor_token_type: 'urn:ietf:params:oauth:token-type:access_token'
 		};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` around lines 380 - 401, The token exchange builds
tokenExchangeParams but never adds the actor token; update the logic in the
token-exchange flow (the block that constructs tokenExchangeParams in the
function that calls createSubjectToken and exchanges it) to add actor_token and
actor_token_type when an actorToken argument is provided: set
tokenExchangeParams.actor_token = actorToken and
tokenExchangeParams.actor_token_type =
'urn:ietf:params:oauth:token-type:access_token' (matching the subject token type
used) so the returned JWT includes the act claim that src/api/context/oidc.ts
expects.
🧹 Nitpick comments (4)
src/api/services/OIDC.ts (4)

41-43: Type guard parameter uses any.

Use unknown for the parameter type to enforce proper type narrowing within the guard.

-export function isValidOIDCUser(user: any): user is OIDCUser {
-	return !!user.sub && !!user.email;
+export function isValidOIDCUser(user: unknown): user is OIDCUser {
+	return typeof user === 'object' && user !== null && 'sub' in user && 'email' in user && !!user.sub && !!user.email;
 }

As per coding guidelines: "NEVER use any type in TypeScript".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` around lines 41 - 43, Change the isValidOIDCUser
parameter from any to unknown and implement proper runtime narrowing: update the
signature to isValidOIDCUser(user: unknown): user is OIDCUser and inside the
function first check that user is a non-null object, then verify it has the
required properties (e.g. 'sub' and 'email') and optionally that those
properties are strings before returning true; reference the OIDCUser type and
the isValidOIDCUser function to locate and update the guard logic.

314-315: Consider adding runtime validation for the M2M token response.

The response.json() result is cast implicitly. Adding basic validation that data.access_token exists and is a string would improve robustness against unexpected API responses.

 	const data = await response.json();
+	if (typeof data?.access_token !== 'string') {
+		throw new Error('Invalid M2M token response: missing access_token');
+	}
 	return data.access_token;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` around lines 314 - 315, The code currently does
await response.json() and returns data.access_token without checking shape; add
runtime validation after calling response.json() to ensure the parsed object has
an access_token property of type string (e.g., check typeof data?.access_token
=== 'string'), throw a descriptive error if missing or invalid, and only then
return the token to prevent downstream failures when response.json() returns an
unexpected shape; update the function in OIDC.ts that performs response.json()
to include this guard around the data and access_token variables.

33-33: Index signature uses any type.

The coding guidelines prohibit any unless unavoidable. Consider using unknown for the index signature, which forces proper type narrowing when accessing dynamic claims.

-	[key: string]: any;
+	[key: string]: unknown;

As per coding guidelines: "NEVER use any type in TypeScript - only use it when absolutely unavoidable with a // TYPE-SAFETY-EXCEPTION: comment explaining why and keeping scope narrow".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` at line 33, Replace the unsafe index signature
"[key: string]: any" in OIDC.ts with a safer type (e.g., "Record<string,
unknown>" or "[key: string]: unknown") and update any places that read
properties from this indexed object to perform explicit type narrowing/casting
(use type guards, typeof checks, or specific casts) before treating values as
strings/numbers/objects; if you truly cannot avoid `any`, add a narrow-scoped
"// TYPE-SAFETY-EXCEPTION:" comment explaining why and limit its use to the
smallest possible symbol.

49-49: Consider Record<string, unknown> for stricter type safety.

While Record<string, any> is common for dynamic OIDC claims, Record<string, unknown> would align better with the coding guidelines and force explicit type checks when accessing properties.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/services/OIDC.ts` at line 49, Change the parameter type of
normalizeOIDCClaims from Record<string, any> to Record<string, unknown> to
improve type safety; update the function signature normalizeOIDCClaims(claims:
Record<string, unknown>): Record<string, any> and then adjust any property
accesses inside the function to perform explicit type checks or narrow with type
assertions (e.g., typeof checks or Array.isArray) before using claim values so
compilation is satisfied and runtime behavior remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/api/services/OIDC.ts`:
- Line 258: Replace the type assertion on remoteUserInfo by using the
isValidOIDCUser type guard to let TypeScript narrow the type: call
isValidOIDCUser(remoteUserInfo) and return remoteUserInfo directly from inside
that conditional branch (and handle the failure path consistently—throw or
return the appropriate error/undefined) instead of using "as OIDCUser";
reference the isValidOIDCUser function and the remoteUserInfo variable in
OIDC.ts to locate where to make this change.
- Around line 248-252: The call that falls back to fetch user info incorrectly
passes the raw access_token as the expectedSubject; update the call to pass the
actual subject (sub) or undefined instead of access_token so fetchUserInfo
receives a user ID, e.g. replace the third argument currently using "sub ??
access_token" with just "sub" (or "sub ?? undefined") and keep the surrounding
use of normalizeOIDCClaims and accessTokenClaims unchanged; check the
identifiers fetchUserInfo, normalizeOIDCClaims, access_token, sub, and
accessTokenClaims when making the change.

---

Duplicate comments:
In `@src/api/services/OIDC.ts`:
- Around line 380-401: The token exchange builds tokenExchangeParams but never
adds the actor token; update the logic in the token-exchange flow (the block
that constructs tokenExchangeParams in the function that calls
createSubjectToken and exchanges it) to add actor_token and actor_token_type
when an actorToken argument is provided: set tokenExchangeParams.actor_token =
actorToken and tokenExchangeParams.actor_token_type =
'urn:ietf:params:oauth:token-type:access_token' (matching the subject token type
used) so the returned JWT includes the act claim that src/api/context/oidc.ts
expects.

---

Nitpick comments:
In `@src/api/services/OIDC.ts`:
- Around line 41-43: Change the isValidOIDCUser parameter from any to unknown
and implement proper runtime narrowing: update the signature to
isValidOIDCUser(user: unknown): user is OIDCUser and inside the function first
check that user is a non-null object, then verify it has the required properties
(e.g. 'sub' and 'email') and optionally that those properties are strings before
returning true; reference the OIDCUser type and the isValidOIDCUser function to
locate and update the guard logic.
- Around line 314-315: The code currently does await response.json() and returns
data.access_token without checking shape; add runtime validation after calling
response.json() to ensure the parsed object has an access_token property of type
string (e.g., check typeof data?.access_token === 'string'), throw a descriptive
error if missing or invalid, and only then return the token to prevent
downstream failures when response.json() returns an unexpected shape; update the
function in OIDC.ts that performs response.json() to include this guard around
the data and access_token variables.
- Line 33: Replace the unsafe index signature "[key: string]: any" in OIDC.ts
with a safer type (e.g., "Record<string, unknown>" or "[key: string]: unknown")
and update any places that read properties from this indexed object to perform
explicit type narrowing/casting (use type guards, typeof checks, or specific
casts) before treating values as strings/numbers/objects; if you truly cannot
avoid `any`, add a narrow-scoped "// TYPE-SAFETY-EXCEPTION:" comment explaining
why and limit its use to the smallest possible symbol.
- Line 49: Change the parameter type of normalizeOIDCClaims from Record<string,
any> to Record<string, unknown> to improve type safety; update the function
signature normalizeOIDCClaims(claims: Record<string, unknown>): Record<string,
any> and then adjust any property accesses inside the function to perform
explicit type checks or narrow with type assertions (e.g., typeof checks or
Array.isArray) before using claim values so compilation is satisfied and runtime
behavior remains unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f000227-5078-4e49-ba4b-86f0a0165a72

📥 Commits

Reviewing files that changed from the base of the PR and between a2d05b9 and 3530e9d.

📒 Files selected for processing (1)
  • src/api/services/OIDC.ts

@Strehk Strehk merged commit 1d483b6 into main Apr 6, 2026
13 checks passed
@Strehk Strehk deleted the auth/replace-zitadel-with-logto branch April 6, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants